-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: add markdown linting and spellcheck #186
docs: add markdown linting and spellcheck #186
Conversation
830e7f7
to
76dd1a1
Compare
Would be good to get some feedback, and if you like it, I'll add some documentation before merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, I think this will certainly help us maintain the quality of the docs :)
This ensures that tox does not use any platform-specific commands.
a77f393
to
c2fb90e
Compare
Rebase due to:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I really like the dictionaries - I feel like we are bulletproof now :D
This PR:
docs/**/*.md
&README.md
.Motivation
We now have CD of our docs from the main branch and we should try and maintain a certainly level of quality:
https://ethereum.github.io/execution-spec-tests/main
External Tooling Required
nvm
to manage and install different node versions, see https://www.freecodecamp.org/news/node-version-manager-nvm-install-guide/.Tooling and Integration with VS Code:
Markdown spellchecking is already covered in VS Code via the Code Spell Checker that we already recommend in our
extensions.json
.Dictionaries: Now "whitelist.txt" is split into 3 separate wordlist files for maintainability. One reason: If you do "Quick Fix" -> "Add word to dictionary 'project-words'", VS Code sorts the wordlist alphabetically, which is not so optimal for our list of opcodes. Pyspelling supports multiple wordlists. flake8-spellcheck works only supports one wordlist file, which must be called
whitelist.txt
. This is generated using thesrc/entry_points/create_whitelist.py
in tox prior to running flake8.Btw, I did check whether we could spellcheck the markdown via the flake8-spellcheck plugin for simplicity, but it was not possible.
I used the vscode-markdownlint for a couple of weeks - it works well. This extension uses the
markdownlint-cli2
and there's a corresponding Github Action that uses this tool.If you use these two plugins and clean-up any errors, then tox should pass.
Soft Fail Mode Rational
It's not ideal to require external (non-native Python) tooling and these tools are only required for people who work heavily on the docs. As such, if these tools are not available, tox prints a hint that they're missing, but will still pass. Most devs will only be working on the tests and should not break the docs or need to change them (docstrings in tests will get spell-checked from flake8; linting the output is more difficult, perhaps there's a flake8 plugin extension?).
Keeping tox cross-platform (hopefully)
Additionally, the tools are executed via Python scripts in
src/entry_points
to avoid using external commands (e.g. bash) in tox to ensure it still runs cross-platform.Github Actions
These checks will always be ran in Github actions: